Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Control panel for simulations #443

Open
wants to merge 47 commits into
base: master
Choose a base branch
from
Open

Control panel for simulations #443

wants to merge 47 commits into from

Conversation

lemniscate8
Copy link
Collaborator

@lemniscate8 lemniscate8 commented Aug 4, 2021

  • Add a Button group from Bootstrap
  • Add a toggle button group that is double click safe
    • Can configure as a cassette style or play/pause style button
    • Can gray out inactive state for stronger visual indication
  • Add a control panel
    • Interface for adding buttons
    • Support different call rates to an simulation (function to run) and viewports (components to redraw)

@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #443 (414e349) into master (08c913f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #443      +/-   ##
==========================================
+ Coverage   82.96%   82.97%   +0.01%     
==========================================
  Files         304      306       +2     
  Lines       37341    37393      +52     
==========================================
+ Hits        30979    31028      +49     
- Misses       6362     6365       +3     
Impacted Files Coverage Δ
include/emp/web/Div.hpp 71.50% <ø> (ø)
include/emp/datastructs/DisjointVariant.hpp 100.00% <100.00%> (ø)
include/emp/prefab/ToggleSwitch.hpp 100.00% <100.00%> (ø)
include/emp/web/Input.hpp 41.13% <100.00%> (ø)
tests/datastructs/DisjointVariant.cpp 100.00% <100.00%> (ø)
include/emp/Evolve/World_select.hpp 96.01% <0.00%> (-0.40%) ⬇️
include/emp/Evolve/World.hpp 96.44% <0.00%> (-0.31%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08c913f...414e349. Read the comment docs.

@lemniscate8 lemniscate8 requested a review from mmore500 August 21, 2021 00:39
* button group group.
* @param btn_group a button group
*/
ButtonGroup & TakeChildren(ButtonGroup & btn_group) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're destroying it, maybe take as an R-value reference?


protected:
/**
* The protected contructor for a ButtonGroup.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention the README.md or somewhere else where the design pattern is explained

include/emp/prefab/ButtonGroup.hpp Show resolved Hide resolved
include/emp/prefab/ButtonGroup.hpp Show resolved Hide resolved
};
}

#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// #ifndef EMP_BUTTON_GROUP_HPP
(here and all the other files)

}

/**
* Get the function to be called when the component toggles
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do end users need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point :D


/**
* Set the function to be called when the component toggles
* @param cb a callback function that accepts a boolean indicating
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a void callback function

@@ -52,6 +57,8 @@ namespace web {
class TableInfo;
class DivInfo : public internal::WidgetInfo {
friend Element; friend Div; friend TableInfo;
friend prefab::ButtonGroup;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do ButtonGroup and ControlPanel need to be friends? What are they needing to access inside of Div???

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They need to be able to construct a DivInfo instance but the constructor is protected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did we get around this with other prefab components? It would be best to try to preserve levilization if possible (although these predeclares aren’t a /huge/ issue).

@@ -305,6 +312,7 @@ namespace web {
// Get a properly cast version of info.
internal::DivInfo * Info() { return (internal::DivInfo *) info; }
const internal::DivInfo * Info() const { return (internal::DivInfo *) info; }
Div(internal::DivInfo * in_info) : WidgetFacet(in_info) { ; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some space, an explanatory comment, and info about where design pattern is explained.

struct Test_Control_Panel : emp::web::BaseTest {
/*
* Creates extra div + the following control panel structure
* +------------------+------+-----+-------------------+ +---+---+---+ +--+
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D | A | B ??? XD

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, C?

Can just add legend below

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not sure what D, A, B and C are

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants